src/main/java/de/tum/cit/aet/artemis/atlas/domain/profile/LearnerProfile.java (2)

42-61: Consider default values for the new feedback fields

The feedback preference fields are well-defined with appropriate column mapping and validation constraints. However, they don't have default initialization values, which could cause unexpected behavior if accessed before being set.

- private int feedbackPracticalTheoretical;
+ private int feedbackPracticalTheoretical = 3; // default middle value

- private int feedbackCreativeGuidance;
+ private int feedbackCreativeGuidance = 3; // default middle value

- private int feedbackFollowupSummary; 
+ private int feedbackFollowupSummary = 3; // default middle value

- private int feedbackBriefDetailed;
+ private int feedbackBriefDetailed = 3; // default middle value

---

90-120: Consider adding validation in setters

The setters are simple and follow the standard pattern. However, for better error handling, consider adding validation in the setters to ensure the values comply with the @min(1) and @max(5) constraints.

public void setFeedbackPracticalTheoretical(int feedbackPracticalTheoretical) {
+    if (feedbackPracticalTheoretical < 1 || feedbackPracticalTheoretical > 5) {
+        throw new IllegalArgumentException("Feedback value must be between 1 and 5");
+    }
    this.feedbackPracticalTheoretical = feedbackPracticalTheoretical;
}

This pattern should be applied to all four setter methods.

src/main/webapp/i18n/de/userSettings.json (1)

11-11: Space in "Lerner Profil" may be unnecessary

In German, compound nouns are typically written as a single word. Consider using "Lernerprofil" instead of "Lerner Profil" for better German localization.

- "learnerProfile": "Lerner Profil",
+ "learnerProfile": "Lernerprofil",

src/main/webapp/app/core/user/settings/learner-profile/learner-profile.component.scss (2)

14-44: Consider alternatives to ::ng-deep for component styling

The use of ::ng-deep is deprecated and will be removed in a future Angular version. While often necessary for styling Angular Material components, it can lead to future maintenance issues.

Consider these alternatives:

1. Use component view encapsulation: ViewEncapsulation.None (less preferred)
2. Create a global theme extension for Material components
3. Use CSS custom properties defined at the component level and consumed inside

Example for option 3:

:host {
  --slider-thumb-size: 24px;
  --slider-track-height: 8px;
  --slider-primary-color: var(--bs-primary);
}

// Then use these variables in your selectors

---

61-63: Use theme variables consistently

The hover state uses a hardcoded color (#007bff) with the darken function, while the rest of the styles use CSS variables (var(--bs-primary)).

For consistency, consider:

-        background-color: darken(#007bff, 5%);
+        background-color: darken(var(--bs-primary), 5%);

Or better yet, use a CSS variable for the hover state:

-        background-color: darken(#007bff, 5%);
+        background-color: var(--bs-primary-dark, darken(var(--bs-primary), 5%));

src/main/webapp/app/learner-profile/service/learner-profile-api.service.ts (1)

12-14: Simplify HTTP request URL

The string template literal with empty interpolation can be simplified. Also, consider adding error handling for this HTTP request.

getLearnerProfileForCurrentUser(): Observable<LearnerProfileDTO> {
-    return this.http.get<LearnerProfileDTO>(`${this.resourceUrl}`);
+    return this.http.get<LearnerProfileDTO>(this.resourceUrl);
}

src/main/java/de/tum/cit/aet/artemis/atlas/dto/LearnerProfileDTO.java (1)

8-8: Add validation or documentation for allowed value ranges

The record declaration is clean, but it would be helpful to document the expected range of values for the feedback fields.

- public record LearnerProfileDTO(long id, int feedbackPracticalTheoretical, int feedbackCreativeGuidance, int feedbackFollowupSummary, int feedbackBriefDetailed) {
+ /**
+  * Learner Profile Data Transfer Object. 
+  * Feedback values are expected to be in the range of 1-5, where:
+  * - 1 represents the first preference (practical, creative, follow-up, brief)
+  * - 5 represents the second preference (theoretical, focused, summary, detailed)
+  */
+ public record LearnerProfileDTO(long id, int feedbackPracticalTheoretical, int feedbackCreativeGuidance, int feedbackFollowupSummary, int feedbackBriefDetailed) {

src/main/webapp/app/core/user/settings/learner-profile/learner-profile.component.ts (3)

46-59: Add loading state indicator during save operation

While the component tracks the saving state with isSaving, it doesn't seem to be used in the UI to provide visual feedback to users during the save operation.

Consider adding a loading spinner or disabling the save button while saving:

// In the template (learner-profile.component.html), you should have something like:
// <button [disabled]="isSaving" (click)="save()">
//   <fa-icon *ngIf="!isSaving" [icon]="faSave"></fa-icon>
//   <span *ngIf="isSaving" class="spinner-border spinner-border-sm" role="status" aria-hidden="true"></span>
//   {{ 'save' | translate }}
// </button>

---

29-29: Improve documentation for hideTooltip method

The purpose of the hideTooltip method isn't clear. If it's intended to prevent tooltips from showing, this should be documented.

-hideTooltip = () => '';
+// Returns empty string to disable default tooltips on sliders
+hideTooltip = () => '';

---

47-48: Use type for subscribe callback parameter

For better type safety, specify the type for the updatedProfile parameter in the subscribe callback.

 this.learnerProfileApiService.putUpdatedLearnerProfile(this.learnerProfile).subscribe({
-    next: (updatedProfile: LearnerProfileDTO) => {
+    next: (updatedProfile: LearnerProfileDTO) => {

Note: This is already correctly typed, so I'm approving this part of the code. The type annotation provides good clarity.

src/main/java/de/tum/cit/aet/artemis/atlas/web/LearnerProfileResource.java (7)

28-30: Consider using an enum for profile value constraints

Using static final integers for MIN and MAX profile values is good, but an enum would provide better type safety and encapsulation of related constants.

-private static final int MIN_PROFILE_VALUE = 1;
-private static final int MAX_PROFILE_VALUE = 5;
+private enum ProfileValueBounds {
+    MIN(1),
+    MAX(5);
+    
+    private final int value;
+    
+    ProfileValueBounds(int value) {
+        this.value = value;
+    }
+    
+    public int getValue() {
+        return value;
+    }
+}

Then update the validation method:

 private void validateProfileField(int value, String fieldName) {
-    if (value < MIN_PROFILE_VALUE || value > MAX_PROFILE_VALUE) {
+    if (value < ProfileValueBounds.MIN.getValue() || value > ProfileValueBounds.MAX.getValue()) {
         throw new BadRequestAlertException(fieldName + " field is outside valid bounds", LearnerProfile.ENTITY_NAME, fieldName.toLowerCase() + "OutOfBounds", true);
     }
 }

---

47-51: Consider improving error message clarity in validation

The current error message doesn't specify the valid range, which would be useful information for clients.

 private void validateProfileField(int value, String fieldName) {
     if (value < MIN_PROFILE_VALUE || value > MAX_PROFILE_VALUE) {
-        throw new BadRequestAlertException(fieldName + " field is outside valid bounds", LearnerProfile.ENTITY_NAME, fieldName.toLowerCase() + "OutOfBounds", true);
+        throw new BadRequestAlertException(fieldName + " field must be between " + MIN_PROFILE_VALUE + " and " + MAX_PROFILE_VALUE, LearnerProfile.ENTITY_NAME, fieldName.toLowerCase() + "OutOfBounds", true);
     }
 }

---

53-59: Add more comprehensive API documentation

The GET endpoint lacks JavaDoc comments that describe what the method does, unlike the PUT endpoint which has detailed documentation.

+/**
+ * GET /learner-profiles : get the {@link LearnerProfile} for the current user.
+ *
+ * @return A ResponseEntity with status 200 (OK) and the current user's learner profile,
+ *         or an error response if the profile cannot be found.
+ */
 @GetMapping("learner-profiles")
 @EnforceAtLeastStudent
 public ResponseEntity<LearnerProfileDTO> getLearnerProfile() {

---

72-81: Consider consolidating validation checks

The method has multiple separate validation checks. Consider consolidating them for better readability and maintainability.

-if (learnerProfileDTO.id() != learnerProfileId) {
-    throw new BadRequestAlertException("Provided learnerProfileId does not match learnerProfile.", LearnerProfile.ENTITY_NAME, "objectDoesNotMatchId", true);
-}
-
-Optional<LearnerProfile> optionalLearnerProfile = learnerProfileRepository.findByUser(user);
-
-if (optionalLearnerProfile.isEmpty()) {
-    throw new BadRequestAlertException("LearnerProfile not found.", LearnerProfile.ENTITY_NAME, "LearnerProfileNotFound", true);
-}
+// Validate request integrity
+if (learnerProfileDTO.id() != learnerProfileId) {
+    throw new BadRequestAlertException("Provided learnerProfileId does not match learnerProfile.", LearnerProfile.ENTITY_NAME, "objectDoesNotMatchId", true);
+}
+
+// Fetch and validate learner profile existence
+LearnerProfile updateProfile = learnerProfileRepository.findByUser(user)
+    .orElseThrow(() -> new BadRequestAlertException("LearnerProfile not found.", LearnerProfile.ENTITY_NAME, "LearnerProfileNotFound", true));

---

83-86: Consider using a batch validation approach

The validation calls are repetitive. Consider using a more elegant approach to validate all fields.

-validateProfileField(learnerProfileDTO.feedbackPracticalTheoretical(), "FeedbackPracticalTheoretical");
-validateProfileField(learnerProfileDTO.feedbackCreativeGuidance(), "FeedbackCreativeGuidance");
-validateProfileField(learnerProfileDTO.feedbackFollowupSummary(), "FeedbackFollowupSummary");
-validateProfileField(learnerProfileDTO.feedbackBriefDetailed(), "FeedbackBriefDetailed");
+// Validate all feedback fields
+Map<String, Integer> fieldsToValidate = Map.of(
+    "FeedbackPracticalTheoretical", learnerProfileDTO.feedbackPracticalTheoretical(),
+    "FeedbackCreativeGuidance", learnerProfileDTO.feedbackCreativeGuidance(),
+    "FeedbackFollowupSummary", learnerProfileDTO.feedbackFollowupSummary(),
+    "FeedbackBriefDetailed", learnerProfileDTO.feedbackBriefDetailed()
+);
+
+fieldsToValidate.forEach(this::validateProfileField);

Remember to update the validateProfileField method signature to accept these parameters.

---

88-92: Consider using a builder pattern or mapping utility

The manual setting of each field is verbose and error-prone. Consider using a mapping utility or builder pattern.

-LearnerProfile updateProfile = optionalLearnerProfile.get();
-updateProfile.setFeedbackPracticalTheoretical(learnerProfileDTO.feedbackPracticalTheoretical());
-updateProfile.setFeedbackCreativeGuidance(learnerProfileDTO.feedbackCreativeGuidance());
-updateProfile.setFeedbackFollowupSummary(learnerProfileDTO.feedbackFollowupSummary());
-updateProfile.setFeedbackBriefDetailed(learnerProfileDTO.feedbackBriefDetailed());
+// Update learner profile fields
+updateProfile = updateLearnerProfileFromDTO(updateProfile, learnerProfileDTO);

// Add this method to the class:
+/**
+ * Updates a LearnerProfile entity with values from DTO
+ *
+ * @param profile The existing profile to update
+ * @param dto The DTO containing new values
+ * @return The updated profile
+ */
+private LearnerProfile updateLearnerProfileFromDTO(LearnerProfile profile, LearnerProfileDTO dto) {
+    profile.setFeedbackPracticalTheoretical(dto.feedbackPracticalTheoretical());
+    profile.setFeedbackCreativeGuidance(dto.feedbackCreativeGuidance());
+    profile.setFeedbackFollowupSummary(dto.feedbackFollowupSummary());
+    profile.setFeedbackBriefDetailed(dto.feedbackBriefDetailed());
+    return profile;
+}

---

93-96: Consider adding transaction management

While saving to the repository, it's a good practice to ensure the operation is handled within a transaction.

+import org.springframework.transaction.annotation.Transactional;

// In the method signature:
+@Transactional
 public ResponseEntity<LearnerProfileDTO> updateLearnerProfile(@PathVariable long learnerProfileId, @RequestBody LearnerProfileDTO learnerProfileDTO) {

However, note that the coding guidelines mention "avoid_transactions", so this suggestion might need further discussion to understand if transactions are handled at a different layer in this application.

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

src/main/webapp/app/core/user/settings/learner-profile/learner-profile.component.scss (3)

29-36: Consider using theme variables for consistent color management.

The hardcoded color values for the slider track elements could be replaced with theme variables for better consistency with the application's theme system, especially since you're already using var(--bs-primary) for active elements.

-                background-color: #e0e0e0 !important;
+                background-color: var(--bs-light) !important;

This would ensure your component adapts properly to any theme changes in the application.

---

50-51: Use theme variables for text colors.

Similar to the previous comment, it's better to use theme variables for text colors rather than hardcoded values like #666.

-        color: #666;
+        color: var(--bs-secondary-text);

---

62-62: Use theme variables instead of hardcoded colors.

Using a hardcoded color and darken function is less maintainable than using theme variables.

-        background-color: darken(#007bff, 5%);
+        background-color: var(--bs-primary-dark, darken(var(--bs-primary), 5%));

src/main/webapp/app/learner-profile/shared/entities/learner-profile.model.ts (1)

1-21: Consider adding type constraints or documentation for valid ranges.

The model looks well-structured with appropriate naming conventions. However, since these feedback preference fields likely have a valid range (possibly 1-5 based on the UI sliders), it would be helpful to either:

1. Add documentation about the valid range
2. Use a more specific type than number to ensure correct usage

+/**
+ * Data Transfer Object for Learner Profile settings
+ * Feedback preferences are on a scale of 1-5, with 3 as the default (neutral) value
+ */
 export class LearnerProfileDTO {
     public id: number;
+    /** Feedback preference from 1 (practical) to 5 (theoretical) */
     public feedbackPracticalTheoretical: number;
+    /** Feedback preference from 1 (creative) to 5 (focused) */
     public feedbackCreativeGuidance: number;
+    /** Feedback preference from 1 (follow-up) to 5 (summary) */
     public feedbackFollowupSummary: number;
+    /** Feedback preference from 1 (brief) to 5 (detailed) */
     public feedbackBriefDetailed: number;

src/main/webapp/i18n/en/learnerProfile.json (1)

5-5: Ensure description matches the translation scope.

The current description is quite general and doesn't specifically mention feedback preferences like the German translation does. Consider making it more specific to align with the actual purpose of these settings.

-            "description": "Customize your learning experience by adjusting your preferences.",
+            "description": "Customize your learning experience by adjusting your feedback style preferences.",

src/main/java/de/tum/cit/aet/artemis/atlas/dto/LearnerProfileDTO.java (1)

10-19: Consider adding documentation for feedback preference values

The of method is well implemented to convert from the domain entity to the DTO. However, consider adding some JavaDoc comments to explain what each feedback preference field represents and what the scale values (1-5) mean.

 public record LearnerProfileDTO(long id, int feedbackPracticalTheoretical, int feedbackCreativeGuidance, int feedbackFollowupSummary, int feedbackBriefDetailed) {

+    /**
+     * Feedback fields represent user preferences on a scale of 1-5 where:
+     * - feedbackPracticalTheoretical: 1=practical, 5=theoretical
+     * - feedbackCreativeGuidance: 1=creative, 5=guided/focused
+     * - feedbackFollowupSummary: 1=follow-up, 5=summary
+     * - feedbackBriefDetailed: 1=brief, 5=detailed
+     */
+
     /**
      * Creates LearnerProfileDTO from given LearnerProfile.
      *
      * @param learnerProfile The given LearnerProfile
      * @return LearnerProfile DTO for transfer
      */

src/main/webapp/app/core/user/settings/learner-profile/learner-profile.component.html (3)

25-32: Standardize binding approach across sliders

The first slider uses separate [value] and (valueChange) bindings while the other sliders (lines 42, 53, 64) use [(ngModel)]. For consistency, use the same approach across all sliders.

<mat-slider min="1" max="5" step="1" [discrete]="true" showTickMarks [displayWith]="hideTooltip">
    <input
        matSliderThumb
-       [value]="learnerProfile.feedbackPracticalTheoretical"
-       (valueChange)="learnerProfile.feedbackPracticalTheoretical = $event"
+       [(ngModel)]="learnerProfile.feedbackPracticalTheoretical"
        (change)="onSliderChange()"
    />
</mat-slider>

---

25-65: Add accessibility attributes to sliders

The sliders are missing accessibility attributes such as aria-label that would improve user experience for those using assistive technologies.

<mat-slider min="1" max="5" step="1" [discrete]="true" showTickMarks [displayWith]="hideTooltip">
    <input
        matSliderThumb
+       [attr.aria-label]="'artemisApp.learnerProfile.feedback.feedbackPracticalTheoretical.title' | translate"
        [value]="learnerProfile.feedbackPracticalTheoretical"
        (valueChange)="learnerProfile.feedbackPracticalTheoretical = $event"
        (change)="onSliderChange()"
    />
</mat-slider>

---

33-69: Consider adding numeric indicators for slider values

The sliders show labels for the extremes (1 and 5), but users might benefit from seeing the current numeric value they've selected. Consider adding a visual indicator of the current value.

<div class="labels">
    <span jhiTranslate="artemisApp.learnerProfile.feedback.feedbackPracticalTheoretical.practical"></span>
+   <span class="current-value">{{ learnerProfile.feedbackPracticalTheoretical }}</span>
    <span jhiTranslate="artemisApp.learnerProfile.feedback.feedbackPracticalTheoretical.theoretical"></span>
</div>

src/main/java/de/tum/cit/aet/artemis/atlas/domain/profile/LearnerProfile.java (1)

90-120: Add documentation to feedback getter/setter methods

Consider adding JavaDoc comments to the getter and setter methods to improve code documentation and explain what each preference represents.

+/**
+ * Gets the feedback preference for practical vs theoretical content.
+ * Lower values (1) indicate preference for practical content, higher values (5) for theoretical.
+ * @return feedback preference value between 1-5
+ */
public int getFeedbackPracticalTheoretical() {
    return feedbackPracticalTheoretical;
}

+/**
+ * Sets the feedback preference for practical vs theoretical content.
+ * @param feedbackPracticalTheoretical value between 1-5
+ */
public void setFeedbackPracticalTheoretical(int feedbackPracticalTheoretical) {
    this.feedbackPracticalTheoretical = feedbackPracticalTheoretical;
}

src/main/java/de/tum/cit/aet/artemis/atlas/web/LearnerProfileResource.java (1)

61-96: Consider reducing duplication in the update operation.

The update method contains redundant code for setting each feedback field. Consider refactoring this to reduce duplication. Additionally, consider adding explicit transaction management for clarity.

+import org.springframework.transaction.annotation.Transactional;

 @PutMapping(value = "learner-profiles/{learnerProfileId}")
 @EnforceAtLeastStudent
+@Transactional
 public ResponseEntity<LearnerProfileDTO> updateLearnerProfile(@PathVariable long learnerProfileId, @RequestBody LearnerProfileDTO learnerProfileDTO) {
     User user = userRepository.getUser();

     if (learnerProfileDTO.id() != learnerProfileId) {
         throw new BadRequestAlertException("Provided learnerProfileId does not match learnerProfile.", LearnerProfile.ENTITY_NAME, "objectDoesNotMatchId", true);
     }

-    Optional<LearnerProfile> optionalLearnerProfile = learnerProfileRepository.findByUser(user);
-
-    if (optionalLearnerProfile.isEmpty()) {
-        throw new BadRequestAlertException("LearnerProfile not found.", LearnerProfile.ENTITY_NAME, "LearnerProfileNotFound", true);
-    }
+    LearnerProfile updateProfile = learnerProfileRepository.findByUserElseThrow(user);

     validateProfileField(learnerProfileDTO.feedbackPracticalTheoretical(), "FeedbackPracticalTheoretical");
     validateProfileField(learnerProfileDTO.feedbackCreativeGuidance(), "FeedbackCreativeGuidance");
     validateProfileField(learnerProfileDTO.feedbackFollowupSummary(), "FeedbackFollowupSummary");
     validateProfileField(learnerProfileDTO.feedbackBriefDetailed(), "FeedbackBriefDetailed");

-    LearnerProfile updateProfile = optionalLearnerProfile.get();
     updateProfile.setFeedbackPracticalTheoretical(learnerProfileDTO.feedbackPracticalTheoretical());
     updateProfile.setFeedbackCreativeGuidance(learnerProfileDTO.feedbackCreativeGuidance());
     updateProfile.setFeedbackFollowupSummary(learnerProfileDTO.feedbackFollowupSummary());
     updateProfile.setFeedbackBriefDetailed(learnerProfileDTO.feedbackBriefDetailed());

     LearnerProfile result = learnerProfileRepository.save(updateProfile);
     return ResponseEntity.ok(LearnerProfileDTO.of(result));
 }

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro